plugins/webhooks/src/modules/webhooks/service.ts (4)

47-52: Add validation for payload before signature creation.

The current implementation doesn't validate that the payload is properly serializable before creating the signature. This could lead to unexpected errors if the payload contains circular references or non-serializable values.

  createHmacSignature(payload: Record<string, unknown>) {
+   // Ensure payload is serializable to prevent unexpected errors
+   try {
+     const stringifiedPayload = JSON.stringify(payload);
+     return Crypto
+       .createHmac("sha256", this.options.secretKey)
+       .update(stringifiedPayload)
+       .digest("hex");
+   } catch (error) {
+     this.logger.error("Failed to create signature for payload", error);
+     throw new Error("Failed to create signature: payload contains non-serializable values");
+   }
-   return Crypto
-     .createHmac("sha256", this.options.secretKey)
-     .update(JSON.stringify(payload))
-     .digest("hex");
  }

---

67-67: Improve webhook signature header with timestamp.

The current implementation only includes the signature hex digest. Industry standard practice is to also include a timestamp to prevent replay attacks. This allows receivers to verify that the webhook was sent within an acceptable time window.

Consider enhancing the signature header format to include a timestamp:

- "x-webhook-signature": this.createHmacSignature(payload),
+ const timestamp = Math.floor(Date.now() / 1000).toString();
+ "x-webhook-signature": `t=${timestamp},v1=${this.createHmacSignature({ ...payload, timestamp })}`,

This would require updating the createHmacSignature method signature and implementation to handle the enhanced payload, but it significantly improves security against replay attacks.

---

88-88: Apply consistent documentation for method parameters.

You've updated the method signature with stronger typing, but it lacks JSDoc documentation that describes the parameters and return values. This would be helpful for developers using this API.

+ /**
+  * Sends webhook events to multiple webhook endpoints
+  * @param webhooks - Array of webhook models to send events to
+  * @param payload - The data payload to send with the webhook
+  * @returns Promise resolving to an array of webhook send responses
+  */
  public async sendWebhooksEvents(webhooks: WebhookModel[], payload: Record<string, unknown>) {

---

123-125: Update test webhook method to use the same payload type.

The testWebhookSubscription method still uses a simple object { test: true } as payload, while other methods now expect Record<string, unknown>. This is technically compatible, but it would be better to be explicit for consistency.

- const response = await this.send(testData, {
-   test: true,
- });
+ const response = await this.send(testData, {
+   test: true,
+   timestamp: new Date().toISOString(),
+ } as Record<string, unknown>);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

plugins/webhooks/README.md (1)

32-32: Prefer formatting consistency and avoid hardcoding secrets
The snippet currently reads:

secretKey:"Your-webhook-secret"

1. Add a space after the colon and a trailing comma for uniform JSON style:

-    secretKey:"Your-webhook-secret"
+    secretKey: "Your-webhook-secret",

2. (Optional) Instead of hardcoding, demonstrate reading the secret from an environment variable to follow security best practices:

-    secretKey: "Your-webhook-secret",
+    secretKey: process.env.WEBHOOK_SECRET_KEY,

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro